Skip to content

fix(desktop): delete provider-backed instances before persona cascade - #3808

Open
jbforge wants to merge 3 commits into
block:mainfrom
jbforge:fix/persona-delete-provider-instances
Open

fix(desktop): delete provider-backed instances before persona cascade#3808
jbforge wants to merge 3 commits into
block:mainfrom
jbforge:fix/persona-delete-provider-instances

Conversation

@jbforge

@jbforge jbforge commented Jul 30, 2026

Copy link
Copy Markdown

Summary

Fixes #3771.

Deleting a persona whose managed-agent instances are provider-deployed always
failed. delete_persona
(desktop/src-tauri/src/commands/personas/mod.rs:113) takes only (id, app)
there is no force parameter — and it hard-rejects the cascade when any target
is remote-deployed, via collect_remote_deployed
(mod.rs:81, matching backend != Local && backend_agent_id.is_some()):

persona {id} has provider-deployed agent instances (…); delete those agent instances first

Both persona-delete call sites went straight to deletePersona without
touching instances first, so they hit that rejection every time:

  • desktop/src/features/agents/ui/usePersonaActions.tshandleDelete
  • desktop/src/features/profile/ui/UserProfilePanel.tsxhandleConfirmDeletePersona

The confirm dialog made it worse by promising the cascade unconditionally, so
the user saw a promise immediately followed by a refusal.

The fix

Delete the persona's instances first, so the cascade finds nothing left to
refuse. delete_managed_agent already has the escape hatch this needs —
force_remote_delete: Option<bool>
(desktop/src-tauri/src/commands/agents.rs:1276) — and the frontend already
wraps it in the right ritual: deleteManagedAgentWithRules sends !shutdown,
presents the orphan-warning confirm, and only then passes
forceRemoteDelete: true.

New deleteManagedAgentsForPersonaWithRules in
features/agents/lib/managedAgentControlActions.ts loops a persona's instances
through that per-agent function. Contract: stop the persona cascade at the
first cancelled or failed instance delete
, so the persona itself is never
removed while instances remain. Instances already deleted when the abort fires
stay deleted — the cascade bounds the damage rather than unwinding it, and it
reports deletedCount so the caller can tell the user about that partial
state. Instances are deduplicated by normalized pubkey so a list carrying the
same instance twice cannot delete it twice.

Provider deployments are never force-deleted silently — every one goes through
the existing orphan-warning confirm, and declining it stops the whole cascade.

The delete dialog now calls out the provider-hosted instances in the cascade as
their own sentence rather than folding them into the relay-archival one. The
count is scoped to instances that are actually deployed
(backend.type === "provider" && backendAgentId), matching both the condition
the backend rejects on and the condition under which
deleteManagedAgentWithRules forces. Deleting removes the local management
record and its key, tombstones the agent's relay record, and archives its relay
identity — it does not tear down the provider deployment. There is no undeploy
operation today (commands/agents.rs:455), so the remote infrastructure keeps
running until it is removed at the provider, and the dialog now says so,
matching the orphan-warning confirm the same flow shows.

The backend guard is deliberately left alone. Its comment states the
invariant: the rejection is a runtime guard, and a compromised IPC caller must
not be able to orphan a live remote deployment. Adding a force flag to
delete_persona would weaken that for no gain, since the frontend can do the
instance deletion explicitly, behind the orphan-warning confirm, rather than
have the backend bypass its own invariant.

On the two loops

features/profile/ui/UserProfilePanelDeletion.ts has its own
persona-instance loop (deleteProfileManagedAgentsForPersona) and this PR
leaves it in place rather than sharing one implementation. Its extra work is
interleaved per instance — it removes each agent from its channels between
deletes — so sharing the loop would mean giving the generic helper a per-agent
callback parameter, and parameterizing a working delete path to save ~20 lines
seemed the worse trade. Both loops now carry a cross-reference comment naming
the other and the shared abort contract, so drift is visible to a reader.

Happy to consolidate them if maintainers would prefer that.

One deliberate asymmetry: the agents-side cascade does not remove instances
from their channels, while the profile variant does. That preserves the
pre-existing behaviour of the persona cascade — delete_persona only ever
tombstoned and archived — so this fix does not quietly widen what deleting a
persona does beyond making it succeed.

A note on the call signature

usePersonaActions.handleDelete now takes the instance-teardown function as a
required parameter, supplied by AgentsView, which already holds both the
persona and managed-agent hooks. That hook is also mounted app-wide by
RequestedAgentCreateDialogs, which never deletes anything, so wiring four new
queries into it to serve one button seemed wrong. Making the parameter required
rather than optional is deliberate: TypeScript now refuses a call that skips
instance teardown, instead of silently degrading back to the bug this PR fixes.

Tests

11 new, all passing.

Cascade (managedAgentControlActions.test.mjs):

  • deletes every instance backed by the persona
  • ignores instances backed by other personas
  • deletes a duplicated pubkey only once
  • stops at a declined orphan confirm — zero instances deleted
  • stops at the first failed instance delete — later instances untouched
  • a declined confirm does not undo earlier deletes — the aborting instance
    is deliberately not first, so the partial teardown is pinned rather than
    hidden by test ordering

The abort tests assert that instances after the stopping point are never
touched, and that deletedCount reports the ones already destroyed.

Dialog copy (PersonaDeleteDialog.test.mjs): singular and plural provider
disclosure, no provider claim when the count is zero, and parity with the
previous output for callers that don't pass the new argument.

Verified locally at the pushed commit: full desktop suite 3846 passed, 0
failed
; just desktop-check exits 0 (biome, the file-size ratchet,
check:px-text, check:pubkey-truncation); just desktop-build
(tsc && vite build) succeeds; and desktop-tauri-check, desktop-tauri-test
and desktop-tauri-test-compiled-flags all pass. desktop-tauri-clippy fails
locally on macOS with three dead_code errors in
src-tauri/src/linux_media.rs — a file this PR does not touch, in a module
whose code is live on the Linux runner CI uses; this PR changes no Rust.

Not covered: there is no E2E test driving the rendered dialog against a
live provider deployment — the confirm path is exercised at the unit level with
a stubbed window.confirm, not through the real dialog. I also could not
verify against an actual provider-backed agent, so the claim that the cascade
now succeeds rests on the traced code path rather than on a reproduction. A
maintainer with a provider deployment to hand could confirm that end to end
quickly.

Related

@jbforge
jbforge requested a review from a team as a code owner July 30, 2026 21:12
@jbforge
jbforge force-pushed the fix/persona-delete-provider-instances branch from 2e23513 to dd45b88 Compare July 30, 2026 21:23
jbforge added 2 commits July 30, 2026 23:24
Deleting a persona whose instances are provider-deployed always failed.
`delete_persona` has no force parameter and hard-rejects the cascade when any
target is remote-deployed, while `delete_managed_agent` does have the
`force_remote_delete` escape hatch. Both persona-delete call sites went
straight to `deletePersona` without touching instances first, so the cascade
hit that rejection every time. The confirm dialog also promised the cascade
unconditionally, so the user saw a promise followed by a refusal.

Delete the persona's managed-agent instances first, through
`deleteManagedAgentWithRules`, so the cascade finds nothing left to refuse.
That routes each provider deployment through the existing orphan-warning
confirm rather than force-deleting it silently. The cascade stops at the first
cancelled or failed instance delete, so the persona is never removed while
instances remain; instances already deleted when it stops stay deleted.

The new `deleteManagedAgentsForPersonaWithRules` sits beside the per-agent
function it wraps. `features/profile` keeps its own loop because its channel
cleanup is interleaved per instance; both loops now carry a cross-reference
comment naming the shared abort contract. The backend guard is deliberately
left alone — its invariant is that a compromised IPC caller must not be able
to orphan a live remote deployment.

The delete dialog now calls out the provider-hosted instances in the cascade as
its own sentence rather than folding them into the relay-archival one. Deleting
removes the local management record and its key, tombstones the agent's relay
record, and archives its relay identity; it does not tear down the provider
deployment, because the protocol has no undeploy operation (deferred to v2,
`commands/agents.rs:455`), so the remote infrastructure keeps running until it
is removed at the provider.

Fixes block#3771

Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com>
Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
Review of the first commit caught a false claim in the copy it added. The
delete dialog told the user that provider-hosted instances "will also be
removed from that provider". Nothing on the delete path contacts a provider:
`delete_managed_agent` stops the local process, drops the record, deletes the
keyring key, then tombstones and publishes a NIP-IA archive request, and
`force_remote_delete` only bypasses the backend's orphan guard. The protocol
has no undeploy operation at all — `commands/agents.rs:455` defers it to v2 —
so the remote deployment keeps running and keeps costing money.

The claim also contradicted the orphan-warning confirm shown moments later in
the same flow, which says the deployment "will keep running". A user who read
the dialog, confirmed, and read past the second warning would believe their
billed infrastructure was gone when it was not. The dialog now says the
deployment is not torn down and keeps running until it is removed at the
provider, and a regression test asserts the copy never claims removal again.

Two smaller corrections from the same review:

The cascade's abort contract was overstated on both loops as "never leaves a
half-torn persona". Aborting stops further deletes; it cannot undo the ones
that already ran. Only provider-deployed instances prompt, so a local instance
visited earlier is already gone when the user declines. The comments now say
that, `deleteManagedAgentsForPersonaWithRules` reports `deletedCount`, and the
Agents view surfaces "Deleted N agent instance(s); kept <persona> because you
cancelled" instead of leaving that destruction silent. A new test puts the
aborting instance second so the partial teardown is pinned rather than hidden
by test ordering.

`handleDeleteInstancesForPersona` deliberately does not remove instances from
their channels, unlike the profile variant; that preserves the pre-existing
behaviour of the persona cascade rather than widening what this fix changes.
The reason is now in the doc comment.

Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com>
Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
@jbforge
jbforge force-pushed the fix/persona-delete-provider-instances branch from dd45b88 to a3a4b91 Compare July 30, 2026 21:25
Review found the PR could not go green: the first commit pushed
`UserProfilePanel.tsx` from 984 to 1004 lines, and the desktop file-size
ratchet caps `src/features` at 1000. CI runs it — `Justfile:118-119`
(`desktop-check`) invokes `pnpm check`, whose `check` script in
`desktop/package.json` chains `check:file-sizes`. Repo convention is to split
the file rather than raise the limit or add an override.

Move the persona-delete cluster into `UserProfilePanelPersonaDelete.ts`, beside
the module that already owns this component's deletion logic: the pending
persona, the two instance counts the confirm dialog discloses, and the
confirmed-delete handler. Only that cluster moves — a thousand-line component
has other seams, but widening the split would put an unrelated refactor inside
a fix PR. `UserProfilePanel.tsx` lands at 964 lines.

The room that frees also closes a gap the same review found. The agents-side
cascade reports a declined confirm to the user; the profile side did not, so an
instance destroyed before the user cancelled vanished with no feedback on a
path that toasts every other outcome. `deleteProfileManagedAgentsForPersona`
now returns `deletedCount` like its sibling, and the profile call site raises
"Deleted N agent instance(s); kept <persona> because you cancelled."

Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com>
Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
@Chessing234

Copy link
Copy Markdown
Contributor

clean ordering fix. worth a regression test that a provider-backed delete never hits the persona cascade path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provider-backed agent delete is routed through persona deletion and fails

2 participants